Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Add basic GetChainInfo method to plugin API #3561

Conversation

clockworkgr
Copy link
Collaborator

No description provided.

@clockworkgr clockworkgr changed the title refactor: ,ake cosmosgen analysis code reusable for plugin system refactor: make cosmosgen analysis code reusable for plugin system Jun 23, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2023

Visit the preview URL for this PR (updated for commit fdf95a3):

https://igntservices-docs--pr3561-feat-plugin-system-c-tfdhxaqw.web.app

(expires Tue, 12 Sep 2023 08:54:04 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 95379efd94dd497aaa37c2d0354e6e2cafca5ec5

@github-actions github-actions bot added component:ci CI/CD workflow and automated jobs. 📖 docs component:cmd type:services Service-related issues. component:packages labels Aug 8, 2023
ignite/cmd/plugin.go Outdated Show resolved Hide resolved

// FilePath is the path of the .proto file where message is defined at.
FilePath string
// File path is the path of the proto file where message is defined.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go's documentation standard:

Suggested change
// File path is the path of the proto file where message is defined.
// FilePath is the path of the proto file where message is defined.


// FilePath is the path of the .proto file where message is defined at.
FilePath string
// File path is the path of the .proto file where message is defined at.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// File path is the path of the .proto file where message is defined at.
// FilePath is the path of the .proto file where message is defined at.

// HighestFieldNumber is the highest field number among fields of the message
// This allows to determine new field number when writing to proto message
HighestFieldNumber int
// Highest field name is the highest field number among fields of the message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Highest field name is the highest field number among fields of the message.
// HighestFieldNumber is the highest field number among fields of the message.

@clockworkgr clockworkgr changed the title refactor: make cosmosgen analysis code reusable for plugin system refactor: Add basic GetChainInfo method to plugin API Sep 4, 2023
}

// Service is an RPC service.
type Service struct {
// Name of the services.
Name string
Name string `json:"name,omitempty"`

// RPC is a list of RPC funcs of the service.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// RPC is a list of RPC funcs of the service.
// RPCFuncs is a list of RPC funcs of the service.

Comment on lines +121 to +124
// Has query indicates if there is a request query.
HasQuery bool `json:"has_query,omitempty"`
// Has body indicates if there is a request payload.
HasBody bool `json:"has_body,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Has query indicates if there is a request query.
HasQuery bool `json:"has_query,omitempty"`
// Has body indicates if there is a request payload.
HasBody bool `json:"has_body,omitempty"`
// HasQuery indicates if there is a request query.
HasQuery bool `json:"has_query,omitempty"`
// HasBody indicates if there is a request payload.
HasBody bool `json:"has_body,omitempty"`

@clockworkgr clockworkgr marked this pull request as ready for review September 4, 2023 11:54
@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Merging #3561 (fdf95a3) into feat/plugin-system-improvements-versioning-analysis (00d5330) will increase coverage by 0.01%.
The diff coverage is 25.68%.

Additional details and impacted files

Impacted file tree graph

@@                                   Coverage Diff                                   @@
##           feat/plugin-system-improvements-versioning-analysis    #3561      +/-   ##
=======================================================================================
+ Coverage                                                25.00%   25.01%   +0.01%     
=======================================================================================
  Files                                                      293      293              
  Lines                                                    23356    23435      +79     
=======================================================================================
+ Hits                                                      5839     5862      +23     
- Misses                                                   16997    17045      +48     
- Partials                                                   520      528       +8     
Files Changed Coverage Δ
ignite/pkg/cosmosanalysis/module/module.go 67.25% <ø> (ø)
ignite/pkg/cosmosclient/mocks/account_retriever.go 26.95% <ø> (ø)
ignite/pkg/cosmosclient/mocks/bank_query_client.go 9.94% <ø> (ø)
ignite/pkg/cosmosclient/mocks/rpc_client.go 7.08% <ø> (ø)
ignite/pkg/cosmosclient/mocks/signer.go 64.51% <0.00%> (-6.92%) ⬇️
ignite/pkg/cosmosgen/generate.go 0.00% <0.00%> (ø)
ignite/pkg/cosmostxcollector/mocks/saver.go 80.64% <0.00%> (-8.65%) ⬇️
...gnite/pkg/cosmostxcollector/mocks/txs_collector.go 80.64% <0.00%> (-8.65%) ⬇️
ignite/pkg/gomodule/gomodule.go 8.97% <0.00%> (ø)
ignite/pkg/protoanalysis/package.go 10.00% <ø> (ø)
... and 8 more

... and 2 files with indirect coverage changes

Comment on lines +270 to +273
c, err = newChainWithHomeFlags(cmd)
if err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this one can be removed, chain has been initialized already within this function definition

Suggested change
c, err = newChainWithHomeFlags(cmd)
if err != nil {
return err
}

chain Chainer
}

func (api clientAPI) GetChainInfo(_ context.Context) (*ChainInfo, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required but this could also be written as:

Suggested change
func (api clientAPI) GetChainInfo(_ context.Context) (*ChainInfo, error) {
func (api clientAPI) GetChainInfo(context.Context) (*ChainInfo, error) {

@jeronimoalbi jeronimoalbi merged commit d3918a7 into feat/plugin-system-improvements-versioning-analysis Sep 5, 2023
29 checks passed
@jeronimoalbi jeronimoalbi deleted the feat/plugin-system-clientAPI branch September 5, 2023 14:00
jeronimoalbi added a commit that referenced this pull request Oct 19, 2023
* chore: rename `types.proto`to `interface.proto`

* feat: add code analizer support

Adds initial analizer support to allow bidirectional communication
between the CLI and the plugins.

The analizer features should be defined and implemented on top of these
changes.

* feat: add analizer support to plugin's interface

* chore: add proto generated code

* chore: generate interface mocks

* refactor: change protocol implementation to support analizer

* chore: fix issue with missing return

* refactor: change plugin command to add analizer to calls

* feat: add proto types required for the cosmos/proto analysis packages

These types allows the implementation of the dependencies analyzer.

* chore: update plugin template to include analizer arguments

* fix: correct type in Analyzer name

* chore: update plugin documentation

* chore: rename analyzer file name because of typo

* fix: correct typo for analyzer names

* chore: update changelog

* test: fix plugin integration test

* test: fix plugin cmd unit tests

* test: fix plugin service unit test

* ci: fix unused argument warning

* refactor: Add basic GetChainInfo method to plugin API (#3561)

* refactor: Analyzer/analizer -> ClientAPI

* refactor: rename proto files and rebuild

* refactor: Add json tags

* wip/refactor: Module analysis

* feat: Add chain reference to plugin ClientAPI

* feat: Complete Dependencies ClientAPI method

* fix: Address review comments

* feat: Remove services/chain dep from pkg/cosmosanalysis as per discussion

* wip: remove deptools install

* feat: package-specific includes

* fix: Replace Module List call with Chain Info call

* chore: Remove chain analysis code

* feat: ChainInfo API example template

* chore: Update template cli reference

* chore: clean up PR

* fix: Address review comments

* fix: address review comments

* fix: address review comments

* fix: Tests and linting

* chore: add changelog

* fix: linting issues

* tests: fix issue with client api in plugin tests

* tests: fix plugin template for integration tests

---------

Co-authored-by: jeronimoalbi <[email protected]>

* chore: correct typos and simplify code

* chore: update pseudo version for plugin's `go.mod` template

* chore: fix typos

* fix: correct call to module dependency resolution

* chore: remove proto file comment

---------

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Clockwork <[email protected]>
Pantani added a commit that referenced this pull request Nov 10, 2023
* chore: update plugin template Go version to 1.20

* docs: add documentation to clarify the plugin cache purpose

* chore: improve plugins code spacing for better redability

* chore: add context support to plugin scaffold

* refactor: move plugin RPC implementation to an `rpc.go` file

* refactor: simplify RPC plugin implementation

* feat: add proto buffer files for the gRPC plugins

* chore: generate plugin files from proto

* chore: remove extra blank line

* chore: add Flags to ExecutedCommand proto definition

* refactor: remove rpc plugin implementation

* refactor: move interface types methods to the generated types

* refactor: move plugin interface into grpc package

* feat: add gRPC plugin implementation

* refactor: change plugin to use gRPC for communication

* refactor: change plugin command to use the new gRPC interface

* refactor: add context to gRPC plugin client communication

* refactor: move plugin interface and gRPC implementation

Done to have a better package API.

* chore: add plugin flag type aliases for v1

* refactor: add flag contructor methods to executed command

* refactor: update plugin template to use gRPC

* test: fixed tests

* chore: move proto flag type enum definition into flag message

* chore: add changelog entry

* chore: correct flag type reference in plugin service tests

* test: improve plugin scaffold test

* test: change flag types

* chore: lower plugin template Go version to 1.19

This is temporary until CI/CD allows Go v1.20

* chore: use `hplugin` alias go hashicorp's go-plugin package

To keep consistency between imports in the plugin package.

* test: fix issue that killed plugins when testing

* test: fix plugin command tests

* fix: correct issue in plugin load

* chore: update plugin documentation

* chore: fix changelog

* chore: add buf to tool dependencies

* feat: add make file targets to generate code from proto

* fix: correct proto file linting issue

* chore: proto file format fix

* ci: add GitHub workflow to check and lint proto files

* ci: disable proto breaking change

It must be disabled until proto files are available in the main branch.

* fix: correct Buf repository name

* test: change plugin integration test to use a local example folder

This was done to allow CI to pass the test without the requirement of
merging the changes into the example plugin repository.

* chore: update Go and CLI versions

Co-authored-by: Danilo Pantani <[email protected]>

* ci: update GitHub workflows to use Go version 1.20

* fix: beam me up Scotty

* test: move command and manifest test to the right location

* test: add plugin gRPC types tests

* ci: correct CI issue with naked return

* fix: correct plugin command path issue

* chore: remove deprecated replace from Go mod

Co-authored-by: Julien Robert <[email protected]>

* chore: change Ignite App handshake config

Co-authored-by: Danilo Pantani <[email protected]>

* feat: add bidirectional communication to plugin system (#3544)

* chore: rename `types.proto`to `interface.proto`

* feat: add code analizer support

Adds initial analizer support to allow bidirectional communication
between the CLI and the plugins.

The analizer features should be defined and implemented on top of these
changes.

* feat: add analizer support to plugin's interface

* chore: add proto generated code

* chore: generate interface mocks

* refactor: change protocol implementation to support analizer

* chore: fix issue with missing return

* refactor: change plugin command to add analizer to calls

* feat: add proto types required for the cosmos/proto analysis packages

These types allows the implementation of the dependencies analyzer.

* chore: update plugin template to include analizer arguments

* fix: correct type in Analyzer name

* chore: update plugin documentation

* chore: rename analyzer file name because of typo

* fix: correct typo for analyzer names

* chore: update changelog

* test: fix plugin integration test

* test: fix plugin cmd unit tests

* test: fix plugin service unit test

* ci: fix unused argument warning

* refactor: Add basic GetChainInfo method to plugin API (#3561)

* refactor: Analyzer/analizer -> ClientAPI

* refactor: rename proto files and rebuild

* refactor: Add json tags

* wip/refactor: Module analysis

* feat: Add chain reference to plugin ClientAPI

* feat: Complete Dependencies ClientAPI method

* fix: Address review comments

* feat: Remove services/chain dep from pkg/cosmosanalysis as per discussion

* wip: remove deptools install

* feat: package-specific includes

* fix: Replace Module List call with Chain Info call

* chore: Remove chain analysis code

* feat: ChainInfo API example template

* chore: Update template cli reference

* chore: clean up PR

* fix: Address review comments

* fix: address review comments

* fix: address review comments

* fix: Tests and linting

* chore: add changelog

* fix: linting issues

* tests: fix issue with client api in plugin tests

* tests: fix plugin template for integration tests

---------

Co-authored-by: jeronimoalbi <[email protected]>

* chore: correct typos and simplify code

* chore: update pseudo version for plugin's `go.mod` template

* chore: fix typos

* fix: correct call to module dependency resolution

* chore: remove proto file comment

---------

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Clockwork <[email protected]>

---------

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Clockwork <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants